Skip to content

DSN: Add cfg.Format method #419

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 3, 2016
Merged

DSN: Add cfg.Format method #419

merged 2 commits into from
Feb 3, 2016

Conversation

julienschmidt
Copy link
Member

This addition allows users to parse a DSN, modify some values and reformat it.
Also it can be used to conveniently build the DSN by filling the Config struct rather then manually building the DSN string.

@julienschmidt julienschmidt added this to the v1.3 milestone Jan 31, 2016
@arnehormann
Copy link
Member

Shouldn't this implement fmt.Stringer and/or fmt.Formatter (maybe with %s skipping the password and %v showing it)?

@julienschmidt
Copy link
Member Author

The method could be renamed to String() in order to implement the fmt.Stringer interface.
But if you use %v, you don't want the DSN string, you want to print the struct. I think the formatting should therefore be done explicitly.

// the driver.
func (cfg *Config) FormatDSN() string {
var buf bytes.Buffer

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about

hasParam := false
addParam := func(key, value string) {
    p := "&"
    if !hasParam {
        p = "?"
        hasParam = true
    }
    buf.WriteString(p + key + "=" + value)
}
//...

to clean this up a little?

Or split it up in multiple Writes to reduce allocs - but that shouldn't matter much here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The boilerplate code here is minimal and is actually useful as it allows to save a few allocations and writes in some cases.

@arnehormann
Copy link
Member

LGTM

julienschmidt added a commit that referenced this pull request Feb 3, 2016
DSN: Add cfg.FormatDSN method
@julienschmidt julienschmidt merged commit acdcb22 into master Feb 3, 2016
@julienschmidt julienschmidt deleted the dsn branch February 3, 2016 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants